Skip to content

[CCXDEV-15259]: Promote InsightsOnDemandDataGather and InsightConfig feature gate to default #2473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[CCXDEV-15259]: Promote InsightsOnDemandDataGather and InsightConfig feature gate to default #2473

wants to merge 4 commits into from

Conversation

opokornyy
Copy link
Contributor

No description provided.

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Sep 11, 2025

Hello @opokornyy! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

@openshift-ci openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 11, 2025
@opokornyy opokornyy changed the title Promote InsightsOnDemandDataGather and InsightConfig feature gate to default [CCXDEV-15259]: Promote InsightsOnDemandDataGather and InsightConfig feature gate to default Sep 11, 2025
@opokornyy
Copy link
Contributor Author

/test verify-feature-promotion

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 21, 2025
@opokornyy opokornyy force-pushed the CCXDEV-15259-insights-promotion branch from 3052b14 to 5d11885 Compare September 22, 2025 07:41
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 22, 2025
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Sep 22, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign deads2k for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@JoelSpeed
Copy link
Contributor

@opokornyy Looking at the data, we have no testing on either metal or SNO, does this feature work on those platforms? Should we be adding testing for them?

@opokornyy
Copy link
Contributor Author

@opokornyy Looking at the data, we have no testing on either metal or SNO, does this feature work on those platforms? Should we be adding testing for them?

We already have metal jobs configured (https://github.com/openshift/release/blob/b7a2a049dacad8d85b856238b0839bb7c276ca76/ci-operator/config/openshift/insights-operator/openshift-insights-operator-release-4.21__periodics.yaml#L329)
). However, I noticed they are missing from the dashboard. I initially thought this was because we were using baremetal instead of metal, so I renamed it, but they still don’t appear in Sippy. I am now trying to fix it... For SNO, we don’t have any tests yet. If required, we can add them.

@opokornyy
Copy link
Contributor Author

@opokornyy Looking at the data, we have no testing on either metal or SNO, does this feature work on those platforms? Should we be adding testing for them?

We already have metal jobs configured (https://github.com/openshift/release/blob/b7a2a049dacad8d85b856238b0839bb7c276ca76/ci-operator/config/openshift/insights-operator/openshift-insights-operator-release-4.21__periodics.yaml#L329) ). However, I noticed they are missing from the dashboard. I initially thought this was because we were using baremetal instead of metal, so I renamed it, but they still don’t appear in Sippy. I am now trying to fix it... For SNO, we don’t have any tests yet. If required, we can add them.

I was checking SNO cluster and the Insights Operator is running there by default, so I am also adding test runs for that platform.

@everettraven
Copy link
Contributor

@opokornyy Thanks for looking into adding those. Both metal (all variants in the promotion requirements) and SNO tests are required.

@everettraven
Copy link
Contributor

everettraven commented Sep 25, 2025 •
edited
Loading

I also dealt with an issue recently where our results weren't showing up in Sippy. We had to make sure our releases section in our configuration for our periodics looked like: https://github.com/openshift/release/blob/531d4d22d226a93f395034cf0b811cd1a00784e1/ci-operator/config/openshift/cluster-authentication-operator/openshift-cluster-authentication-operator-release-4.21__periodics.yaml#L36-L45

@BaiyangZhou
Copy link

/retest

@opokornyy opokornyy force-pushed the CCXDEV-15259-insights-promotion branch from 5d11885 to d72fdf4 Compare October 6, 2025 07:36
@opokornyy
Copy link
Contributor Author

/retest

@opokornyy
feat(insights): remove feature gate
d7b9492 
Signed-off-by: Ondrej Pokorny <[email protected]>
@opokornyy
feat(insights): update hack/update-payload-crds.sh
5430d00 
Remove the v1alpha2 from the payload and let the
script add the v1 instead for insights CRDs.

Signed-off-by: Ondrej Pokorny <[email protected]>
@opokornyy
feat(insights): run make generate
96b47c4 
Signed-off-by: Ondrej Pokorny <[email protected]>
@opokornyy opokornyy force-pushed the CCXDEV-15259-insights-promotion branch from d72fdf4 to 96b47c4 Compare October 14, 2025 07:20
@opokornyy
Copy link
Contributor Author

/hold

The Insights Operator needs to be updated first - openshift/insights-operator#1159

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 14, 2025
@opokornyy
Copy link
Contributor Author

/test verify-feature-promotion

@opokornyy
feat: add DataGather to payload
0e5d556 
Signed-off-by: Ondrej Pokorny <[email protected]>
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Oct 16, 2025

@opokornyy: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gcp 3052b14 link false /test e2e-gcp
ci/prow/verify-crd-schema 0e5d556 link true /test verify-crd-schema
ci/prow/verify-feature-promotion 0e5d556 link true /test verify-feature-promotion
ci/prow/okd-scos-e2e-aws-ovn 0e5d556 link false /test okd-scos-e2e-aws-ovn

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@everettraven everettraven Awaiting requested review from everettraven

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

Assignees

No one assigned

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@opokornyy @JoelSpeed @everettraven @BaiyangZhou @openshift-merge-robot